-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
108 add buncreatinine chart #153
Conversation
…://github.com/SafetyGraphics/nepExplorer into 108-add-buncreatinine-chart
…://github.com/SafetyGraphics/nepExplorer into 108-add-buncreatinine-chart
…tyGraphics/nepExplorer into 108-add-buncreatinine-chart
…tyGraphics/nepExplorer into 108-add-buncreatinine-chart
…tyGraphics/nepExplorer into 108-add-buncreatinine-chart
save_adlb - add BUN/CREAT updates meta_nepExplorer - add BUN/CREAT updates and tidy up ALB/CREAT ratio patient_profile_mod - update for drawBunCreat use the BUN/CREAT ratio patient_profile_charts - update for drawBunCreat use the BUN/CREAT ratio create_nepexplorer_app - add BUN/CREAT mapping examples/safetyGraphics_demo_app - add the ALB/CREAT and BUN/CREAT records and update units for associated tests examples/standalone_app - add BUN/CREATE mapping man/drawBunCreat - add documentation
- Warning: file=inst/examples/safetyGraphics_demo_app.R,line=14,col=3,[object_length_linter] Variable and function names should not be longer than 30 characters. Warning: file=R/patient_profile_charts.R,line=410,col=1,[trailing_blank_lines_linter] Trailing blank lines are superfluous. Warning: file=R/patient_profile_mod.R,line=104,col=51,[trailing_whitespace_linter] Trailing whitespace is superfluous.
Currently we have pkgdown workflow failing because we have stored docs there which are not built by pkgdown - nepExplorer/docs' is non-empty and not built by pkgdown. Do we want to keep these documents here or move them outside the repo? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Lovemore-Gakava I can see the chart in standa lone app and it looks great - nice work on this! Got a few changes/questions in here for your review
PARAMCD == "ALB" ~ convert_alb_to_mgdl(BASE), | ||
TRUE ~ BASE | ||
), | ||
CHG = case_when( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the same changes are being applied to base and aval, i don't think you'd need a unit conversion on CHG, I think that'd stay the same number since it's same division - double check my thinking though @Lovemore-Gakava, it's the morning and my mind is still getting started
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pburnsdata - I appreciate your feedback. The update extends beyond the ratio calculation. It aims to maintain consistency across all variables derived from or associated with BASE or AVAL/LBSTRESN, including CHG, A1LO, A1HI, and others. This ensures that users have reliable data for other various uses outside the BUN/CREAT chart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
o I see, the is raw CHG, idk why I thought this was fold change - sounds good!
data-raw/save_adlb.R
Outdated
mutate(STRESN = ifelse(TEST == "Creatinine" & STRESU == "μmol/L", STRESN / 88.4, # 1 mg/dL creatinine = 88.4 µmol/L | ||
ifelse(TEST == "Blood Urea Nitrogen" & STRESU == "mmol/L", STRESN * 2.86, STRESN) # 1 mmol/L BUN = 2.8 mg/dL BUN | ||
), # Convert μmol/L to mg/dL and mmol/L to mg/dL | ||
STRESU = ifelse(TEST %in% c("Creatinine", "Blood Urea Nitrogen", "Albumin"), "mg/dL", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldn't we need the albumin conversion to mg/dL that you have in the app example code here if we're changing it's unit to mg/dL?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - it's been updated. To be consistent with the ISG demo app.
PARAM == "Blood Urea Nitrogen (mmol/L)" ~ "Blood Urea Nitrogen (mg/dL)", | ||
PARAM == "Albumin (g/L)" ~ "Albumin (mg/dL)", | ||
TRUE ~ PARAM | ||
) # todo: consider removing the units from the PARAM column - if this is implemented also update meta_nepExplorer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I don't think it affects the app in anyway, but could cause confusion in the documentation and examples if users see that and think "oh I need to have units in the param" when they don't. I created #155 for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - lets discuss this further when we look at #155, as we might need to be consistent across the application such as what users see under filters and what is shown on the chart. For example Creatinine (mg/dL)
vs Creatinine
R/patient_profile_charts.R
Outdated
adlb_norm <- adlb %>% | ||
filter(.data[[settings$measure_col]] == settings$measure_values[["BUN/CREAT"]]) | ||
|
||
uacr_unit <- unique(adlb_norm[[settings$unit_col]]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking this unit variabel name should be different since no uacr involved, also I think on safetygraphics app example it gives an error for this chart because there's no unit, we may want to cahnge this to check for units because the if(length) statement below crashes it. I think albumin/creat chart needs the same change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Preston - uacr_unit references updated.
@Lovemore-Gakava good point on the docs, they look like legacy reference material from back when Spencer was working on this, please delete them as part of your PR here to clear up the docs folder. I've palced them here for now: https://github.com/SafetyGraphics/nepExplorer/wiki/Clinical-Reference-Material |
tidyr::spread(TEST, STRESN) %>% | ||
mutate(STRESN = `Blood Urea Nitrogen` / Creatinine, | ||
TEST = "Blood Urea Nitrogen/Creatinine", | ||
STRESU = "Ratio") %>% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wodnering if we need to say anything beyond 'ratio', like i could see users asking "ratio of what". For example, how by looking at the chart can you know the units of the creatinine and BUN? Idk what best practice is, may can ask Jim - could be a separate issue we can make if that makes sense to you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed - I've opened a new issue #157 and lets ask Jim for best practice.
- uuppdated adlb and related files to include alb converted from g/L to mg/dl - removed documents from doc folder - added comment s to explain conversions in safetyGraphics demo app.
- uuppdated adlb and related files to include alb converted from g/L to mg/dl - removed documents from doc folder - added comment s to explain conversions in safetyGraphics demo app.
Merge branch '108-add-buncreatinine-chart' of https://github.com/SafetyGraphics/nepExplorer into 108-add-buncreatinine-chart # Conflicts: # R/patient_profile_charts.R
Merge branch '108-add-buncreatinine-chart' of https://github.com/SafetyGraphics/nepExplorer into 108-add-buncreatinine-chart # Conflicts: # R/patient_profile_charts.R
…tyGraphics/nepExplorer into 108-add-buncreatinine-chart
uacr_unit <- unique(adlb_norm[[settings$unit_col]]) | ||
|
||
if (is.null(adlb_norm[[settings$unit_col]]) || all(adlb_norm[[settings$unit_col]] == "")) { | ||
uacr_unit <- "Ratio" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
love it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @Lovemore-Gakava !
Added the graph of BUN/serum creatinine over time, with two threshold lines, one at 10 and one at 20.
Examples:
examples/standalone_app - should now show the additional plot
examples/safetyGraphics_demo_app - when the ISG is launched go to mapping tab